Conversation
WalkthroughChangesCrop normalization
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/main/resources/META-INF/resources/frontend/src/image-crop.tsx`:
- Around line 144-157: Update the onImageLoad normalization to preserve
explicitly supplied x and y coordinates by removing the unconditional
makeAspectCrop and centerCrop calls. Move px-to-% conversion into a useEffect
that observes the crop state and image dimensions, converting programmatic
server updates before passing the crop to ReactCrop while leaving percentage
crops unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 44225dc7-6409-410f-952f-89ba91e1c764
📒 Files selected for processing (3)
src/main/java/com/flowingcode/vaadin/addons/imagecrop/Crop.javasrc/main/java/com/flowingcode/vaadin/addons/imagecrop/ImageCrop.javasrc/main/resources/META-INF/resources/frontend/src/image-crop.tsx
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/main/resources/META-INF/resources/frontend/src/image-crop.tsx (1)
271-278: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winPrevent
IndexSizeErrorcrash on zero-dimension crops.If the crop region has a width or height of zero (e.g., from an uninitialized state or programmatic update),
outWidthoroutHeightwill evaluate to0. CallingdrawImagewith a zero source width or height throws anIndexSizeError(orInvalidStateError) in browsers, which crashes the script execution and prevents subsequent logic from running. Consider adding an early return to handle this gracefully.🛡️ Proposed fix
const outWidth = Math.round(ccrop.width); const outHeight = Math.round(ccrop.height); + if (outWidth <= 0 || outHeight <= 0) { + return; + } + // Setting canvas dimensions resets the 2D context, so it must happen // before any drawing/clipping state is configured below. canvas.width = outWidth; canvas.height = outHeight;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/resources/META-INF/resources/frontend/src/image-crop.tsx` around lines 271 - 278, Add an early return in the crop-rendering flow after computing outWidth and outHeight, before assigning canvas dimensions or calling drawImage, when either dimension is zero or otherwise non-positive. Preserve the existing rendering path for positive dimensions and allow subsequent logic to continue without throwing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/main/resources/META-INF/resources/frontend/src/image-crop.tsx`:
- Around line 271-278: Add an early return in the crop-rendering flow after
computing outWidth and outHeight, before assigning canvas dimensions or calling
drawImage, when either dimension is zero or otherwise non-positive. Preserve the
existing rendering path for positive dimensions and allow subsequent logic to
continue without throwing.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 808376d8-5c44-4867-9897-a49725681424
📒 Files selected for processing (3)
src/main/java/com/flowingcode/vaadin/addons/imagecrop/Crop.javasrc/main/java/com/flowingcode/vaadin/addons/imagecrop/ImageCrop.javasrc/main/resources/META-INF/resources/frontend/src/image-crop.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/main/java/com/flowingcode/vaadin/addons/imagecrop/ImageCrop.java



Configured
pxcrops mapped to the exported image via rendered (on-screen) pixels, so the output size varied with how the browser scaled the image at crop time (e.g. a 500×500 px crop could export at ~500 or ~667 px). This makes the mapping deterministic.Changes
%(resolution-independent):onChange/onCompletenow use react-image-crop'spercentCrop.onImageLoadnormalizes the configured crop against the image's natural size; apxcrop is interpreted as source (natural) pixels._updateCroppedImagemaps the crop withconvertToPixelCropagainstnaturalWidth/naturalHeight, dropping the rendered→natural rescaling.ResizeObserver/resizeCropworkaround (a%crop needs no rescaling on layout changes) and guardmakeAspectCropagainst an unset aspect.Close #33
Summary by CodeRabbit
Bug Fixes
Documentation
%(resolution-independent) versuspx(based on the image’s natural/source pixels).